Skip to content

ref(utils): simplify useDimensions types by removing type parameter#112460

Merged
JoshuaKGoldberg merged 2 commits intomasterfrom
use-dimension-simplification
Apr 9, 2026
Merged

ref(utils): simplify useDimensions types by removing type parameter#112460
JoshuaKGoldberg merged 2 commits intomasterfrom
use-dimension-simplification

Conversation

@JoshuaKGoldberg
Copy link
Copy Markdown
Member

@JoshuaKGoldberg JoshuaKGoldberg commented Apr 8, 2026

Following up on #112374, I noted two things that could be simplified with useDimensions:

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 8, 2026
@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as ready for review April 8, 2026 14:58
@JoshuaKGoldberg JoshuaKGoldberg requested review from a team as code owners April 8, 2026 14:58
@JoshuaKGoldberg JoshuaKGoldberg requested review from nsdeschenes and removed request for a team April 8, 2026 14:58
Copy link
Copy Markdown
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tend to default to an options bag to make refactoring easier, but we still have a second param to use for any future options here. Nice refactor!

@ryan953
Copy link
Copy Markdown
Member

ryan953 commented Apr 8, 2026

The RTL hook helpers work better with an options bag, they can track types better. In the past we've made everything use options objects like this to make refactors easier and to simplify so cases like that RTL example are not special but more of a default.

@JoshuaKGoldberg
Copy link
Copy Markdown
Member Author

JoshuaKGoldberg commented Apr 8, 2026

Ok I'll go ahead and put back the options object. 👍

@JoshuaKGoldberg JoshuaKGoldberg changed the title ref(utils): simplify useDimensions runtime and types ref(utils): simplify useDimensions types by removing type parameter Apr 8, 2026
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b34d1b0. Configure here.

Comment thread static/app/utils/useDimensions.tsx
@JoshuaKGoldberg JoshuaKGoldberg merged commit 7f2ec20 into master Apr 9, 2026
67 checks passed
@JoshuaKGoldberg JoshuaKGoldberg deleted the use-dimension-simplification branch April 9, 2026 11:58
george-sentry pushed a commit that referenced this pull request Apr 9, 2026
…112460)

Following up on #112374, I noted two things that could be simplified
with `useDimensions`:

* ~Runtime: it takes in an options object, but there's only one
property. This means every call creates a new JS object. This is a
little papercut for performance (almost certainly not impactful on its
own, but they add up).~ This is intentional and preferred; reverted.
* Types: its type parameter is unnecessary, and only bypasses
[`@typescript-eslint/no-unnecessary-type-parameters`](https://typescript-eslint.io/rules/no-unnecessary-type-parameters)
because it forwards the type parameter to `RefObject`
(https://typescript-eslint.io/rules/no-unnecessary-type-parameters/#limitations)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants